Skip to content

feat(#5316): add collapsible health details with localStorage persistence#5364

Open
SteKoe wants to merge 7 commits into
masterfrom
feat/5316-allow-to-collapse-health-details
Open

feat(#5316): add collapsible health details with localStorage persistence#5364
SteKoe wants to merge 7 commits into
masterfrom
feat/5316-allow-to-collapse-health-details

Conversation

@SteKoe
Copy link
Copy Markdown
Contributor

@SteKoe SteKoe commented May 15, 2026

This pull request improves the HealthDetails component by adding a collapsible details section for health information, enhancing accessibility, and ensuring the collapsed state is persisted per instance. It also updates tests to cover the new behavior and adds internationalized strings for the toggle button in multiple languages.

HealthDetails Component Enhancements:

  • Added a collapsible section for health details, including a toggle button with ARIA attributes for accessibility, and persisted the collapsed state in localStorage per instance and health group. [1] [2]
  • Updated the details layout for better alignment and visual clarity, and passed the instance prop to child health-details components. [1] [2] [3] [4]

Testing Improvements:

  • Added comprehensive tests for the collapsible details functionality, including ARIA attributes, toggle behavior, persistence in localStorage, and correct handling of child health components. [1] [2] [3] [4]

Internationalization:

Bildschirmaufnahme.2026-05-15.um.21.37.36.mov

@SteKoe SteKoe requested a review from a team as a code owner May 15, 2026 19:43
@SteKoe SteKoe force-pushed the feat/5316-allow-to-collapse-health-details branch 3 times, most recently from 736495f to 4d4f64a Compare May 17, 2026 16:19
SteKoe added 7 commits May 17, 2026 18:19
- Remove redundant onMounted (isCollapsed already initialised synchronously)
- Guard toggleCollapsed: skip localStorage write when instance.id is absent
- Introduce safeNameId for HTML id attributes and aria-controls (names with spaces/colons/slashes produced invalid ids)
- Guard encodeURIComponent(name ?? '') against null/undefined name
- Fix duplicate child :key to include loop index, preventing wrong instance reuse
- Guard v-else-if object branch with !== null to handle null detail values
- Fix sba-icon-button PropType to [Object, Array, String] so string icon names pass Vue runtime validation
…n, extract safeDetailId

- details v-for :key now includes loop index to handle duplicate detail names
- health.details || health.components replaced with ?? and validated as plain object,
  eliminating false fallthrough on falsy health.details values and both sources
  being shared via a single healthEntries computed
- Inline detail.name.replace() repeated three times replaced with safeDetailId(name, idx)
  helper that also falls back to 'detail_N' when all characters are special
…t row alternation

The Instance node always received index=0, and each health group node also
defaulted to index=0, so all top-level rows rendered with the same bg-white
background. Now the Instance node is explicitly :index="0" and each group
node receives :index="groupIdx + 1", producing correct even/odd alternation
across all sibling health-details rows.
… alternation)

index + 1 was passed to all siblings, giving every child the same index and
therefore the same background colour. Introduced a separate 'depth' prop for
the recursion guard (depth < 10) and pass ':index="idx"' to children so each
sibling gets its own position-based index, producing correct even/odd alternation
at every level of the health component tree.
@SteKoe SteKoe force-pushed the feat/5316-allow-to-collapse-health-details branch from 4d4f64a to 7e880bd Compare May 17, 2026 16:19
@cdprete
Copy link
Copy Markdown
Contributor

cdprete commented May 17, 2026

Nice @SteKoe.

Just one question: is there a reason why the icon is green when the details are expanded? Will its color reflect that status (even if, then, it's clearly visible it's not the same green) or what's the idea behind its coloring scheme?

I find that green a bit confusing without additional context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants